Accept validation: allow +json structured syntax types#165
Accept validation: allow +json structured syntax types#165brandonbloom wants to merge 2 commits intoapple:mainfrom
Conversation
|
I didn't realize the suffix behavior is defined in an RFC, thanks for sharing! I agree we should improve this and handle Now, I think we also need to adjust the client response content type checking logic here: We'll need to be careful when multiple JSON-ish content types are specified, the method linked above does create a specific priority based on parameters, and so on. So we should integrate the suffix handling there. |
|
Thanks, great catch. I updated the client-side content-type matching path as suggested. New commit makes these changes:
|
|
We are also using a backend which uses @czechboy0 I think this might be waiting on a review, is this something you plan on looking at? |
czechboy0
left a comment
czechboy0
left a comment
There was a problem hiding this comment.
Thanks @brandonbloom - I'm still wrapping my head around how we want this to behave, but added a few questions to allow us to move this forward 🙂 Thanks!
| let jsonWith2Params = OpenAPIMIMEType("application/json; charset=utf-8; version=1")! | ||
| let jsonWith1Param = OpenAPIMIMEType("application/json; charset=utf-8")! | ||
| let json = OpenAPIMIMEType("application/json")! | ||
| let anyJsonStructuredSyntax = OpenAPIMIMEType("application/*+json")! |
There was a problem hiding this comment.
Can you also add application/problem+json and then test that application/json is compatible with it in both directions? Not just when an explicit wildcard is used.
| else { return .incompatible(.subtype) } | ||
| let receivedTypeLowercased = receivedType.lowercased() | ||
| let expectedTypeLowercased = expectedType.lowercased() | ||
| guard receivedTypeLowercased == expectedTypeLowercased else { return .incompatible(.type) } |
There was a problem hiding this comment.
Is this right? Should text/foo+json not match application/json?
3 participants
Motivation
Converter.validateAcceptIfPresentvalidates the requestAcceptheader against the server’s chosen responseContent-Type. Today this is strict (exacttype/subtypematch,type/*, or*/*), which means a common real-world pairing fails:Accept: application/jsonContent-Type: application/problem+jsonHTTP content negotiation does not standardize suffix-aware matching for structured syntax suffixes, so treating
application/jsonas compatible withapplication/*+jsonis a server policy choice. However, it is widely expected in practice (especially around Problem Details), and the strict behavior forces downstream servers to add middleware workarounds (e.g. mutateAcceptor avoid emittingapplication/problem+json) just to prevent a runtime error.Modifications
OpenAPIMIMEType.satisfies(acceptValue:)to add an additional compatibility rule for structured syntax suffixes:Accept: application/jsonas compatible withContent-Type: application/problem+json(and otherapplication/*+jsontypes with+json).Accept: application/*+jsonas compatible withapplication/problem+json(and alsoapplication/json).Tests/OpenAPIRuntimeTests/Conversion/Test_Converter+Server.swiftundertestValidateAccept.Result
The runtime’s
Acceptvalidation no longer throws for common JSON-based structured media types likeapplication/problem+jsonwhen clients sendAccept: application/json, reducing downstream middleware/workarounds while keeping existing exact and wildcard matching behavior.Test Plan
swift test --filter Test_ServerConverterExtensions.testValidateAccept